Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[audit] #5: Cache expires to save gas #51

Merged
merged 1 commit into from
Jul 9, 2024
Merged

[audit] #5: Cache expires to save gas #51

merged 1 commit into from
Jul 9, 2024

Conversation

stevieraykatz
Copy link
Collaborator

From Spearbit:

Description
nameExpires[id] has been read multiple times from storage. To save gas it can be cached.

Recommendation
cache nameExpires[id] in a stack variable:

function renew(uint256 id, uint256 duration) external live onlyController returns (uint256) {
    uint256 nameExpiry = nameExpires[id];
    if (nameExpiry + GRACE_PERIOD < block.timestamp) revert NotRegisteredOrInGrace(id);

    nameExpiry += duration;
    nameExpires[id] = nameExpiry;
    emit NameRenewed(id, nameExpiry);
    return nameExpiry;
}

forge snapshot --diff .gas-review:

test_reverts_whenNotInGracePeriod() (gas: -6 (-0.004%)) 
test_reverts_whenNotRegistered() (gas: -6 (-0.010%)) 
test_constructor_setsTheRootNodeOwner() (gas: -27 (-0.211%)) 
test_renewsOwnershipSuccessfully_whenInGracePeriod() (gas: -456 (-0.276%)) 
test_renewsOwnershipSuccessfully_whenNotExpired() (gas: -456 (-0.276%)) 
test_constructor() (gas: -8061 (-29.447%)) 
test_constructor() (gas: 1149708 (4199.847%)) 
Overall gas change: 1140696 (4.930%)

@stevieraykatz stevieraykatz merged commit 7c9cb7f into main Jul 9, 2024
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant